Skip to content

win32/dllmain.c: drop some unused code #16353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 13, 2024
Merged

win32/dllmain.c: drop some unused code #16353

merged 1 commit into from
Oct 13, 2024

Conversation

DanielEScherzer
Copy link
Member

The code block is guarded by #if 0, and even if it wasn't it is a switch that only contains breaks, i.e. it wouldn't actually do anything if enabled.

@DanielEScherzer
win32/dllmain.c: drop some unused code
8f6e25e 
The code block is guarded by `#if 0`, and even if it wasn't it is a switch that
only contains breaks, i.e. it wouldn't actually do anything if enabled.
devnexen
devnexen approved these changes Oct 11, 2024
View reviewed changes
Copy link
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems no longer justifiable since 25afbdb indeed cc @cmb69

@cmb69
Copy link
Member

cmb69 commented Oct 11, 2024

Relevant commit: 6e172f0.

I don't mind having this code. After all, the file is small, rarely touched, and one can see that the reason switch is deliberately omitted. It's okay for me to drop that code, though.

@DanielEScherzer
Copy link
Member Author

DanielEScherzer commented Oct 13, 2024 •
edited
Loading

Relevant commit: 6e172f0.

I don't mind having this code. After all, the file is small, rarely touched, and one can see that the reason switch is deliberately omitted. It's okay for me to drop that code, though.

Sorry, but I don't see the reason the switch is omitted - is this the "prepared" comment? What is being prepared? Even if the switch was no longer omitted the code wouldn't do anything...

cmb69
cmb69 approved these changes Oct 13, 2024
View reviewed changes
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DanielEScherzer, this is a standard code sequence for DllMain(). It's okay to drop it; if we need it in the future, we can bring it back.

@devnexen devnexen merged commit 150599e into php:master Oct 13, 2024
9 of 10 checks passed
@devnexen
Copy link
Member

Thanks !

@DanielEScherzer DanielEScherzer deleted the patch-2 branch October 13, 2024 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cmb69 cmb69 cmb69 approved these changes

@devnexen devnexen devnexen approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DanielEScherzer @cmb69 @devnexen